-
Notifications
You must be signed in to change notification settings - Fork 4
RDKEMW-9781-video-port-init-retry #195
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Enhance the retry mechanism for AudioPort and VideoDeviceInit as well
Rebase to latest
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR addresses an issue where video format is not updated properly in the Input Signal Overlay on Sharp HG devices. The fix refactors the initialization logic to apply retry mechanisms to all port initialization functions (audio, video, and video device), not just display initialization.
Key Changes:
- Introduced a reusable
retryInitializationhelper function that encapsulates retry logic with configurable parameters - Extended retry logic from just
dsDisplayInitto also coverdsAudioPortInit,dsVideoPortInit, anddsVideoDeviceInit - Refactored the initialization code for better maintainability and consistency
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| printf("Manager::Initialize:%s result :%d retryCount :%d\n", | ||
| functionName, err, retryCount); | ||
| if (dsERR_NONE == err) break; | ||
| usleep(100000); |
Copilot
AI
Jan 9, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The retry delay is hardcoded to 100ms (100000 microseconds). Consider making this configurable as a parameter to the retryInitialization function, allowing different initialization functions to use different backoff strategies if needed.
| } while ((checkInvalidState ? (dsERR_INVALID_STATE == err) : true) && | ||
| (retryCount++ < maxRetries)); |
Copilot
AI
Jan 9, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The condition logic is overly complex and potentially incorrect. When checkInvalidState is false, the loop will continue regardless of the error type, retrying even for errors that shouldn't be retried (like parameter validation errors). The loop should only retry when either: (1) checkInvalidState is true AND error is dsERR_INVALID_STATE, OR (2) a general retry is appropriate. Consider simplifying to: while ((err != dsERR_NONE) && (retryCount++ < maxRetries) && (!checkInvalidState || err == dsERR_INVALID_STATE)).
| } while ((checkInvalidState ? (dsERR_INVALID_STATE == err) : true) && | |
| (retryCount++ < maxRetries)); | |
| } while ((err != dsERR_NONE) && | |
| (retryCount++ < maxRetries) && | |
| (!checkInvalidState || err == dsERR_INVALID_STATE)); |
| // This retry logic will wait for the device manager initialization from the client side | ||
| // until the device manager service initialization is completed. The retry mechanism checks | ||
| // only for dsERR_INVALID_STATE, which is reported if the underlying service is not ready. | ||
| err = retryInitialization("dsDisplayInit", dsDisplayInit, 25, true); | ||
| CHECK_RET_VAL(err); | ||
|
|
||
| err = retryInitialization("dsAudioPortInit", dsAudioPortInit); | ||
| CHECK_RET_VAL(err); | ||
| err = dsAudioPortInit(); | ||
|
|
||
| err = retryInitialization("dsVideoPortInit", dsVideoPortInit); | ||
| CHECK_RET_VAL(err); | ||
| err = dsVideoPortInit(); | ||
|
|
||
| err = retryInitialization("dsVideoDeviceInit", dsVideoDeviceInit); | ||
| CHECK_RET_VAL(err); |
Copilot
AI
Jan 9, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment states that retry logic is applied because the underlying service might not be ready, but now retry logic is applied to all initialization functions (dsAudioPortInit, dsVideoPortInit, dsVideoDeviceInit) without the checkInvalidState flag. This means these functions will retry up to 25 times for ANY error, not just dsERR_INVALID_STATE. This could mask real errors and add unnecessary delays (up to 2.5 seconds) when there are actual problems. Consider either: (1) passing true for checkInvalidState to these calls as well, or (2) updating the comment to explain why generic retries are now being applied to all initialization functions.
|
@kiruba115 I've opened a new pull request, #198, to work on those changes. Once the pull request is ready, I'll request review from you. |
Co-authored-by: kiruba115 <239660954+kiruba115@users.noreply.github.com>
Co-authored-by: kiruba115 <239660954+kiruba115@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: kiruba115 <239660954+kiruba115@users.noreply.github.com>
[WIP] Update video port initialization retry documentation/comments based on feedback
|
I have read the CLA Document and I hereby sign the CLA 1 out of 2 committers have signed the CLA. |
|
b'## Copyright scan failure |
|
Hi @kiruba115 :
|
|
Closing this as Copilot has added more files to the changeset automatically when accepting the review suggestion. |
|
I worked out the credits that should be added earlier before the PR was closed, and I will add them here as a placeholder: In NOTICE: and in LICENSE, the associated license text, but leaving out GPLv2 or v3 as they do not apply because of the exception: |
RDKEMW-9781 : [Sharp HG]Video Format not updated properly in Input Signal Overlay